-
Notifications
You must be signed in to change notification settings - Fork 215
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix #673 and #677, update global locks #678
Fix #673 and #677, update global locks #678
Conversation
Only marked as draft because it requires a baseline which is not yet in |
CCB 2020-12-09 APPROVED
|
Removes the signal mask updates from the POSIX global lock (not needed). Adds a condition variable to the structure, which can be used to directly wake up a waiting task rather than requiring that task to poll the global state.
1412ff1
to
a41f748
Compare
Rebased to main. Also fixed an intermittent issue I found in testing where the mutex would not be released. There is a test case where a task gets deleted immediately after creation, and sometimes this would happen while it was still initializing and holding the table mutex. Solution is to add a cleanup handler around the pthread_cond_wait just like bin sems use, so if the task is deleted, the mutex also gets unlocked. |
@jphickey I resolved a small conflict here, I'll do a squash-commit on this unless you want to rebase and force push. |
Yes, I recall a small conflict with the #endif comment. Does it really need a rebase though? Can't we just make a normal merge commit with the conflict resolved? Do you want me to do that in the integration candidate? I don't like squashes at this point because it makes it hard to track what branches in my fork can be deleted (i.e. there is no specific git record of the merge, and |
I suggested a rebase to avoid having f5b84da in the commit history. Regarding the squash, I think github gives me the option to delete the branch after merging |
No worries about f5b84da we can avoid this either way. Regarding deleting branches I also need to delete them on my local machine - not just the fork. I very much prefer not to break I can easily/quickly push a merge commit to integration-candidate that fixes it. Or alternatively if you really prefer a rebase I can do that too but I'd prefer to do it myself as it keeps my local branches in sync with my fork branches. |
f5b84da
to
a41f748
Compare
Fix nasa#666, alignment of CMD/TLM message definitions
Describe the contribution
Reworks the POSIX global lock implementation.
Does not change the POSIX signal mask when locking/unlocking the global. Aside from being unnecessary, there was also a race condition in here (Fixes POSIX unnecessarily setting signal masks in global lock #673)
Adds a condition variable to the global lock structure. In the event that there is more than task competing for access to the same object, this allows the second task to be woken immediately instead of polling for a change (Fixes Implement better way to wait for status change #677).
Testing performed
Build and run all unit tests
Build and sanity check CFE
Expected behavior changes
No longer changing signal masks repeatedly/unexpectedly. (may be relevant to some BSP/driver developers)
System(s) tested on
Ubuntu 20.04
RTEMS 4.11
Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.